-
-
Notifications
You must be signed in to change notification settings - Fork 0
Add PostMountMethodSignatureRule
#10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
23d0d14 to
3546da6
Compare
bb09df6 to
77f7112
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a new PostMountMethodSignatureRule to validate methods with the #[PostMount] attribute and refactors the existing PreMountMethodSignatureRule to allow more flexible method signatures. Both rules now use PHPStan's reflection API for more robust validation and allow optional parameters (0 or 1 array parameter) and multiple return types (array, void, or array|void).
Key changes:
- Added
PostMountMethodSignatureRulewith validation for public visibility, optional array parameter, and array/void/array|void return types - Refactored
PreMountMethodSignatureRuleto useReflectionProviderinstead of AST-only validation and updated signature requirements to match PostMount - Reorganized and consolidated test fixtures for better clarity and coverage
Reviewed changes
Copilot reviewed 32 out of 34 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Rules/TwigComponent/PreMountMethodSignatureRule.php | Refactored to use reflection-based validation, now allows optional parameter and void/array return types |
| src/Rules/TwigComponent/PostMountMethodSignatureRule.php | New rule implementing same validation logic as PreMount for PostMount lifecycle hook |
| tests/Rules/TwigComponent/PreMountMethodSignatureRule/PreMountMethodSignatureRuleTest.php | Reorganized tests into focused test methods per validation concern |
| tests/Rules/TwigComponent/PreMountMethodSignatureRule/Fixture/*.php | Consolidated and renamed test fixtures for clarity (e.g., InvalidNotPublic → InvalidVisibility) |
| tests/Rules/TwigComponent/PostMountMethodSignatureRule/**/*.php | Complete test suite for new PostMount rule with all validation scenarios |
| tests/Rules/TwigComponent/ClassNameShouldNotEndWithComponentRule/**/*.php | Renamed fixture files and fixed class names to be more descriptive |
| tests/Rules/TwigComponent/ForbiddenAttributesPropertyRule/Fixture/ComponentWithCustomAttributesProperty.php | Fixed class name to match filename |
| README.md | Added documentation for PostMountMethodSignatureRule with examples; updated PreMountMethodSignatureRule description (though inaccurate) |
| phpstan.dist.neon | Removed missingType.return and return.unusedType from ignore list (improved code quality) |
| ecs.php | Generalized ProtectedToPrivateFixer skip pattern to cover all test fixtures |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * @implements Rule<Node\Stmt\Class_> |
Copilot
AI
Nov 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent PHPDoc annotation. This rule uses @implements Rule<Node\Stmt\Class_> while PreMountMethodSignatureRule and other rules in the codebase use @implements Rule<Class_> (with the import use PhpParser\Node\Stmt\Class_). For consistency with other rules, change to @implements Rule<Class_> and add the import use PhpParser\Node\Stmt\Class_ at the top.
| $reflClass = $this->reflectionProvider->getClass($node->namespacedName->toString()); | ||
| $errors = []; |
Copilot
AI
Nov 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Inconsistent variable declaration order between lines 43-44. In PreMountMethodSignatureRule (line 44-45), $errors is declared before $reflClass, but here it's reversed. For consistency, consider using the same order: $errors = []; followed by $reflClass = ....
| $reflClass = $this->reflectionProvider->getClass($node->namespacedName->toString()); | |
| $errors = []; | |
| $errors = []; | |
| $reflClass = $this->reflectionProvider->getClass($node->namespacedName->toString()); |
77f7112 to
055bb6b
Compare
No description provided.